Skip to content

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Aug 29, 2022

This is blocking the clippy sync (#101140). One of the lint passes contains a Cell in order to make lifetimes work. It could be worked around, but this is the easier change to make if there are no objections.

Rational for removing the requirement

  • All lint pass methods take &mut self arguments.
  • Many passes depend on running is visitor order.
  • Lint passes are created on demand so they're only ever stored in a local.
  • Send is enough to lint different passes in parallel.

LintStore remains Sync with this. The constructor functions it contains still maintain their Sync requirement.

r? rust-lang/compiler

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 29, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2022
@compiler-errors
Copy link
Member

Does the parallel compiler build with this change?

@Jarcho
Copy link
Contributor Author

Jarcho commented Aug 29, 2022

It builds locally. CI is also building the parallel compiler is it not?

@compiler-errors
Copy link
Member

I don't think CI builds the parallel compiler, like it doesn't even build more than just the x86_64-unknown-linux-gnu compiler in stage2. But I did check that enabling the parallel compiler and building this branch works, so this is fine then.

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 29, 2022

📌 Commit 74f2d58 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2022
@Jarcho
Copy link
Contributor Author

Jarcho commented Aug 29, 2022

The failure in #101140 only happens when building the parallel compiler, so I had assumed that was what CI was doing.

@compiler-errors
Copy link
Member

Guess it does then 👍

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2022
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#100898 (Do not report too many expr field candidates)
 - rust-lang#101056 (Add the syntax of references to their documentation summary.)
 - rust-lang#101106 (Rustdoc-Json: Retain Stripped Modules when they are imported, not when they have items)
 - rust-lang#101131 (CTFE: exposing pointers and calling extern fn is just impossible)
 - rust-lang#101141 (Simplify `get_trait_ref` fn used for `virtual_function_elimination`)
 - rust-lang#101146 (Various changes to logging of borrowck-related code)
 - rust-lang#101156 (Remove `Sync` requirement from lint pass objects)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bf42ba4 into rust-lang:master Aug 30, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants